Skip to content

Move Surface, draw, transform and image docs to stubs #3389

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 5, 2025

Conversation

zoldalma999
Copy link
Member

More progress on #2757

@zoldalma999 zoldalma999 requested a review from a team as a code owner March 19, 2025 11:24
@Starbuck5
Copy link
Member

This PR has a few merge conflicts.

@Starbuck5 Starbuck5 added the docs label Mar 22, 2025
@Starbuck5 Starbuck5 marked this pull request as draft April 1, 2025 06:24
@zoldalma999 zoldalma999 marked this pull request as ready for review April 11, 2025 20:31
@ankith26
Copy link
Member

I did a side by side comparison of the old docs and the new docs

Surface

  • get_frect does not have a versionadded (in both versions) despite it being a relatively new method. Not the fault of this PR, just something I noticed.
  • _pixels_address is documented before this PR, but this PR removes documentation for it. I think this is something to be fixed here.

Tranform

  • pygame.transform.threshold has an example section before this, which is not in this PR. If this change was intentional, I think it is fine.

@zoldalma999
Copy link
Member Author

get_frect does not have a versionadded (in both versions) despite it being a relatively new method. Not the fault of this PR, just something I noticed.

Fixed

_pixels_address is documented before this PR, but this PR removes documentation for it. I think this is something to be fixed here.

Good catch, forgot that private members need to be added to the rst explicitly. Fixed.

pygame.transform.threshold has an example section before this, which is not in this PR. If this change was intentional, I think it is fine.

Yep, that is intentional. It was a test that was included anyway, I don't think that is a good example. But of course can be added back if others disagree.

Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the PR 🎉

@Starbuck5
Copy link
Member

I also combed through it, found a few things.

Surface.get_locks() return type looks strange
Surface.get_at() changes what previously seemed to be a note into a changed in 1.9 notice, drops a created in 1.9 notice.
image.load_basic has a line break between the 3rd and 4th sentence not in current docs

Copy link
Member

@Starbuck5 Starbuck5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I trust you to resolve those notes as practical, if worth it. But I'm also going to approve, thanks for the PR!

@ankith26
Copy link
Member

ankith26 commented May 1, 2025

Surface.get_at() changes what previously seemed to be a note into a changed in 1.9 notice, drops a created in 1.9 notice.

I noticed that too, and assumed it's a fix made in this PR

@zoldalma999 zoldalma999 added this to the 2.5.4 milestone May 5, 2025
@zoldalma999 zoldalma999 merged commit 638745c into pygame-community:main May 5, 2025
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants